Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing requirements.txt and venvs for Python tutorials #563

Merged
merged 12 commits into from
Oct 19, 2024

Conversation

NiklasVin
Copy link
Collaborator

Adds the missing requirements.txt files mentioned in #547 and modifies the run scripts accordingly.

I followed the approach suggested here: #547 (comment) to prevent redundant requirement files.
This concerns the following tutorials:

  • partitioned-heat-conduction
  • partitioned-heat-conduction-complex
  • partitioned-heat-conduction-overlap
  • volume-coupled-diffusion
  • oscillator-overlap
  • oscillator

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not tried to run or check if each requirements.txt included all and the correct dependencies, but the changes look good.

Thanks also for catching and fixing some small inconsistencies (e.g., forgotten close_log, or initializing the venv before starting to log)!

Copy link
Member

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! This really gives a very good overview of the current state and where we have to clarify some questions. I added some comments to point to the open issues. I will briefly summarize here:

  • Generally restrict numpy.
  • Is mpi4py actually a fenicsprecice dependency? Move it there?
  • If pyprecice is a fenicsprecice dependency move it there
  • Find a good approach for FEniCS

I would suggest to merge #564 and #557 independently from this PR because I expect that we are quicker with these smaller PRs.

elastic-tube-1d/fluid-python/requirements.txt Outdated Show resolved Hide resolved
elastic-tube-1d/solid-python/requirements.txt Outdated Show resolved Hide resolved
elastic-tube-3d/solid-fenics/requirements.txt Outdated Show resolved Hide resolved
elastic-tube-3d/solid-fenics/requirements.txt Outdated Show resolved Hide resolved
elastic-tube-3d/solid-fenics/requirements.txt Outdated Show resolved Hide resolved
@NiklasVin
Copy link
Collaborator Author

If pyprecice is a fenicsprecice dependency move it there

I saw that pyprecice is a dependency of fenicsprecice, so I am going to remove it from the requirements👍

Copy link
Member

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

Just a few minor (non-critical) point left. We can merge this PR as it is or you can quickly take care about the fixes.

I will merge this PR latest over the weekend.

@BenjaminRodenberg BenjaminRodenberg merged commit a67bad1 into precice:develop Oct 19, 2024
1 check passed
@BenjaminRodenberg BenjaminRodenberg mentioned this pull request Oct 21, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants